Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove busywork left over from flush promises. #234

Merged
merged 2 commits into from
Mar 26, 2018

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Mar 26, 2018

Motivation:

The Socket channels and PendingDatagramWritesManager had a bunch of
overhead left over from when they were handling flush promises. This was
only adding to code complexity and perf cost.

Modifications:

Removed the promise label in markPendingWritePoint.

Reworked the code to stop expecting write promises in
PendingDatagramWritesManager.

Result:

Smaller, faster code.

Motivation:

The Socket channels and PendingDatagramWritesManager had a bunch of
overhead left over from when they were handling flush promises. This was
only adding to code complexity and perf cost.

Modifications:

Removed the promise label in markPendingWritePoint.

Reworked the code to stop expecting write promises in
PendingDatagramWritesManager.

Result:

Smaller, faster code.
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Mar 26, 2018
@Lukasa Lukasa requested review from normanmaurer and weissi March 26, 2018 12:54
@normanmaurer normanmaurer added this to the 1.4.0 milestone Mar 26, 2018
Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, that looks good, thanks!

@Lukasa Lukasa merged commit 0a23199 into apple:master Mar 26, 2018
@Lukasa Lukasa deleted the cb-clean-up-missing-promises branch March 26, 2018 16:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants